fix(meshing): clear dm_plex_gmsh options after each import (spacedim leak)#238
Open
lmoresi wants to merge 1 commit into
Open
fix(meshing): clear dm_plex_gmsh options after each import (spacedim leak)#238lmoresi wants to merge 1 commit into
lmoresi wants to merge 1 commit into
Conversation
…(+ cache guard)
A (root): the dm_plex_gmsh_* options are global, import-time scratch on the PETSc
options DB. A manifold-surface generator sets dm_plex_gmsh_spacedim=2 and, if it
raises before cleanup (e.g. SegmentedSphericalSurface2D errors in
set_periodicity AFTER the import), the option leaks — and the NEXT gmsh import
(say a SphericalShell) is read with spacedim=2, producing a 2-D mesh in a 3-D
cdim. This surfaced as cross-test pollution: after test_0760_mesh_ot_adapt, a
fresh SphericalShell loaded as 2-D ('cannot reshape array of size 856 into
shape (3)') and the corrupt 2-D mesh was even written to the shell's cache.
Fix centrally in _from_gmsh: clear the whole dm_plex_gmsh_* namespace in a
finally after every import (they are meaningful only for the single import that
follows), so a value set for one import cannot leak into the next — on success
or failure.
B (defence-in-depth): in the Mesh constructor, validate the DM coordinate array
is divisible by cdim before reshaping; if not, raise a clear, actionable error
('stale/corrupt cached mesh ... delete .meshes/*.msh(.h5)') instead of an opaque
numpy reshape failure.
Pre-existing core-meshing bug (independent of #202); exposed by the surface
tests building a SphericalShell right after a 2-D manifold generator. Full
tier-A green; verified: failed manifold construct → next SphericalShell is
correct 3-D; no false positives on valid meshes.
Underworld development team with AI support from Claude Code
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a global PETSc options leak during Gmsh mesh import (dm_plex_gmsh_*) that could silently corrupt subsequent imports (notably spacedim leaking from manifold/surface generators into later 3D meshes), and adds a clearer failure mode when cached meshes have inconsistent coordinate data.
Changes:
- Clear
dm_plex_gmsh_*PETSc options in afinallyblock after every_from_gmsh()import to prevent cross-import option leakage. - Add a coordinate-array size vs
cdimvalidation inMesh.__init__to raise an actionable error instead of a NumPy reshape exception.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # before cleanup leaks it, and the stale option silently corrupts the NEXT gmsh | ||
| # import (e.g. a subsequent SphericalShell is read with spacedim=2 → a 2-D mesh | ||
| # in a 3-D cdim → a "cannot reshape" crash, or a corrupt cache). ``_from_gmsh`` | ||
| # therefore clears the whole namespace after every import (see below). |
Comment on lines
+785
to
+787
| f"cache written at a different space dimension). Delete the " | ||
| f"cached '.meshes/*.msh' and '.msh.h5' for this mesh and " | ||
| f"regenerate." |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A pre-existing core-meshing bug: the
dm_plex_gmsh_*options are global,import-time scratch on the PETSc options database, but they were not cleared
after a gmsh import. A manifold-surface generator sets
dm_plex_gmsh_spacedim = 2and, if it raises before cleanup (e.g.SegmentedSphericalSurface2Derrors inset_periodicityafter the import),that option leaks — and the next gmsh import (say a
SphericalShell) is readwith
spacedim=2, producing a 2-D mesh in a 3-Dcdim.Symptom (found via cross-test pollution): after a 2-D manifold generation, a
fresh
SphericalShellloads as 2-D and crashes withcannot reshape array of size 856 into shape (3)— and the corrupt 2-D mesh iseven written to the shell's on-disk cache, so it persists.
Fix
dm_plex_gmsh_*namespace in afinallyafterevery import, centrally in
_from_gmsh. These options are meaningful only forthe single import that follows, so a value set for one import can no longer
leak into the next — on success or failure.
Meshconstructor, validate the DMcoordinate array is divisible by
cdimbefore reshaping; if not, raise aclear, actionable error (stale/corrupt cache → delete
.meshes/*.msh(.h5))instead of an opaque numpy reshape failure.
Validation
Verified on the integration branch where it was found: a failed manifold
construct followed by a
SphericalShellnow yields a correct 3-D mesh; fulllevel_1 + tier_ais green; no false positives on valid meshes.Cherry-picked from validated work on the manifold/extract_surface integration
branch (PR #237), where it gates the surface-extraction tests' CI.
Underworld development team with AI support from Claude Code